Extract UI components into shared @repo/shadcn package and add Base UI wrappers#111
Extract UI components into shared @repo/shadcn package and add Base UI wrappers#111naymurdev wants to merge 2 commits into
@repo/shadcn package and add Base UI wrappers#111Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (39)
✅ Files skipped from review due to trivial changes (19)
📝 WalkthroughWalkthroughAdds a Radix-based Accordion and Base UI wrapper; creates many base-ui re-export shim modules and new ui/* entrypoints; updates package dependencies; and replaces the local Button with re-exports from ChangesComponent Library Expansion: Accordion and Base UI Integration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~40 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
⚔️ Resolve merge conflicts
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/shadcn/src/base-ui/alert-dialog.tsxESLint skipped: missing config or dependency (missing-dependency). The ESLint configuration references a package that is not available in the sandbox. packages/shadcn/src/base-ui/alert.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox. packages/shadcn/src/base-ui/checkbox.tsxESLint skipped: the ESLint configuration for this file references a package that is not available in the sandbox.
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 202b4c7eff
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| Button.displayName = 'Button'; | ||
|
|
||
| export { Button, buttonVariants }; | ||
| export { Button, buttonVariants } from '@repo/shadcn' |
There was a problem hiding this comment.
uilayouts button variant
Re-exporting the shared package button drops the app-specific uilayouts variant that this file used to define. The website still renders <Button variant='uilayouts'> in apps/ui-layout/components/website/copy-page.tsx lines 82 and 136, and the shared packages/shadcn/src/ui/button.tsx variant map has no uilayouts entry, so those copy-page controls lose their intended dark/light border styling at runtime rather than behaving like the previous local button.
Useful? React with 👍 / 👎.
| "@base-ui-components/react": "^1.0.0-beta.3", | ||
| "@radix-ui/react-accordion": "^1.2.4", | ||
| "@radix-ui/react-collapsible": "^1.1.4", | ||
| "@radix-ui/react-separator": "^1.1.4" |
There was a problem hiding this comment.
These newly added packages are not represented anywhere in the committed bun.lock (I checked with rg for @base-ui-components/react, @radix-ui/react-accordion, @radix-ui/react-collapsible, and @radix-ui/react-separator). In environments that install with bun install --frozen-lockfile—whose help says it disallows lockfile changes—the install cannot materialize the new dependencies required by the new accordion import, so the lockfile needs to be regenerated and committed with the package.json changes.
Useful? React with 👍 / 👎.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
packages/shadcn/package.json (1)
25-25: ⚡ Quick winPin beta dependency to an exact version.
^1.0.0-beta.3allows upgrades to any stable 1.x release (e.g., 1.0.0, 1.1.0), risking unexpected API drift in this foundational package. Pin to an exact version until Base UI reaches stable 2.0.0.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@packages/shadcn/package.json` at line 25, The dependency entry "`@base-ui-components/react`": "^1.0.0-beta.3" should be pinned to an exact version to avoid accidental upgrades; update the package.json dependency for `@base-ui-components/react` by removing the caret and using the exact version string "1.0.0-beta.3" (so it reads "`@base-ui-components/react`": "1.0.0-beta.3"), then run your package manager install to lock the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/shadcn/src/index.tsx`:
- Around line 3-4: The top-level exports in packages/shadcn/src/index.tsx are
causing duplicate exports: remove the explicit accordion re-export and eliminate
the conflicting base-ui barrel to avoid duplicate symbols (e.g.,
BaseUiAccordion, Button, Badge, Tooltip, Command, Popover); specifically delete
the line exporting './base-ui/accordion' and remove (or replace) the line
exporting './base-ui' so you only export the intended modules directly (or only
export the base-ui barrel), ensuring no duplicate re-exports of ui/button and
other identical identifiers.
In `@packages/shadcn/src/ui/accordion.tsx`:
- Around line 43-49: The consumer-facing className is being applied to the inner
wrapper instead of the content root; change the placement so
AccordionPrimitive.Content receives the merged classes (including
'overflow-hidden text-sm data-[state=closed]:animate-accordion-up
data-[state=open]:animate-accordion-down' and the incoming className via cn) and
remove className from the inner div that currently wraps children so that
consumers can override the content root; update the component that renders
AccordionPrimitive.Content (keeping ref, {...props}, children) to merge
cn(className, ...) onto AccordionPrimitive.Content instead of the inner <div>.
---
Nitpick comments:
In `@packages/shadcn/package.json`:
- Line 25: The dependency entry "`@base-ui-components/react`": "^1.0.0-beta.3"
should be pinned to an exact version to avoid accidental upgrades; update the
package.json dependency for `@base-ui-components/react` by removing the caret and
using the exact version string "1.0.0-beta.3" (so it reads
"`@base-ui-components/react`": "1.0.0-beta.3"), then run your package manager
install to lock the change.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: c5854704-a847-4f28-98d6-f066edc146b8
📒 Files selected for processing (31)
apps/ui-layout/components/website/ui/button.tsxapps/ui-layout/package.jsonpackages/shadcn/package.jsonpackages/shadcn/src/base-ui/accordion.tsxpackages/shadcn/src/base-ui/aspect-ratio.tsxpackages/shadcn/src/base-ui/badge.tsxpackages/shadcn/src/base-ui/button.tsxpackages/shadcn/src/base-ui/calendar.tsxpackages/shadcn/src/base-ui/card.tsxpackages/shadcn/src/base-ui/charts.tsxpackages/shadcn/src/base-ui/command.tsxpackages/shadcn/src/base-ui/dialog.tsxpackages/shadcn/src/base-ui/drawer.tsxpackages/shadcn/src/base-ui/dropdown.tsxpackages/shadcn/src/base-ui/form.tsxpackages/shadcn/src/base-ui/hover-card.tsxpackages/shadcn/src/base-ui/index.tsxpackages/shadcn/src/base-ui/input.tsxpackages/shadcn/src/base-ui/label.tsxpackages/shadcn/src/base-ui/navigation-menu.tsxpackages/shadcn/src/base-ui/plastic-button.tsxpackages/shadcn/src/base-ui/popover.tsxpackages/shadcn/src/base-ui/scroll-area.tsxpackages/shadcn/src/base-ui/slider.tsxpackages/shadcn/src/base-ui/switch.tsxpackages/shadcn/src/base-ui/tabs.tsxpackages/shadcn/src/base-ui/toast.tsxpackages/shadcn/src/base-ui/tooltip.tsxpackages/shadcn/src/index.tsxpackages/shadcn/src/ui/accordion.tsxpackages/shadcn/src/ui/index.tsx
| export * from './base-ui/accordion' | ||
| export * from './base-ui' |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if base-ui/index.tsx re-exports accordion (would cause duplicate)
echo "=== Checking base-ui/index.tsx for accordion re-export ==="
cat packages/shadcn/src/base-ui/index.tsx 2>/dev/null || echo "File not found"
echo ""
echo "=== Checking base-ui modules for pass-through re-exports from ui ==="
# Look at a sample base-ui module to see if it re-exports with same names
rg -n "export.*from.*['\"]\.\.\/ui" packages/shadcn/src/base-ui/ --type tsx --type ts 2>/dev/null || \
rg -n "export.*from.*['\"]\.\.\/ui" packages/shadcn/src/base-ui/
echo ""
echo "=== Checking ui barrel for exported symbols ==="
cat packages/shadcn/src/ui/index.tsx 2>/dev/null | head -30Repository: ui-layouts/uilayouts
Length of output: 1298
🏁 Script executed:
#!/bin/bash
echo "=== Checking what ui/button exports ==="
cat packages/shadcn/src/ui/button/index.tsx 2>/dev/null || echo "Not found"
echo ""
echo "=== Checking what base-ui/button exports ==="
cat packages/shadcn/src/base-ui/button/index.tsx 2>/dev/null || echo "Not found"
echo ""
echo "=== Checking what ui/badge exports ==="
cat packages/shadcn/src/ui/badge/index.tsx 2>/dev/null || echo "Not found"
echo ""
echo "=== Checking what base-ui/badge exports ==="
cat packages/shadcn/src/base-ui/badge/index.tsx 2>/dev/null || echo "Not found"Repository: ui-layouts/uilayouts
Length of output: 275
🏁 Script executed:
#!/bin/bash
echo "=== Directory structure of packages/shadcn/src/ ==="
ls -la packages/shadcn/src/
echo ""
echo "=== Content of ui directory ==="
ls packages/shadcn/src/ui/ 2>/dev/null || echo "ui not found"
echo ""
echo "=== Content of base-ui directory ==="
ls packages/shadcn/src/base-ui/ 2>/dev/null | head -20 || echo "base-ui not found"
echo ""
echo "=== Files in ui directory (recursive, limiting to depth 2) ==="
find packages/shadcn/src/ui -maxdepth 2 -type f -name "*.tsx" -o -name "*.ts" 2>/dev/null | head -20Repository: ui-layouts/uilayouts
Length of output: 1797
🏁 Script executed:
#!/bin/bash
echo "=== ui/button.tsx ==="
cat packages/shadcn/src/ui/button.tsx
echo ""
echo "=== base-ui/button.tsx ==="
cat packages/shadcn/src/base-ui/button.tsxRepository: ui-layouts/uilayouts
Length of output: 2175
🏁 Script executed:
#!/bin/bash
echo "=== ui/accordion.tsx ==="
cat packages/shadcn/src/ui/accordion.tsx
echo ""
echo "=== base-ui/accordion.tsx ==="
cat packages/shadcn/src/base-ui/accordion.tsxRepository: ui-layouts/uilayouts
Length of output: 3671
Remove the redundant accordion export and the conflicting base-ui barrel.
Lines 3–4 cause export collisions:
-
Duplicate accordion export: Line 3 exports
BaseUiAccordionfrom./base-ui/accordion, and line 4 re-exports it via./base-ui(which includesexport * from './accordion'). -
Conflicting base-ui re-exports: Base-ui modules re-export identifiers from ui with identical names (e.g.,
export * from '../ui/button'). Lines 1 and 4 together create duplicate exports ofButton,Badge,Tooltip,Command,Popover, and others, preventing bundler compilation.
Remove line 3 and either remove line 4 or line 1, depending on the intended export strategy.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/shadcn/src/index.tsx` around lines 3 - 4, The top-level exports in
packages/shadcn/src/index.tsx are causing duplicate exports: remove the explicit
accordion re-export and eliminate the conflicting base-ui barrel to avoid
duplicate symbols (e.g., BaseUiAccordion, Button, Badge, Tooltip, Command,
Popover); specifically delete the line exporting './base-ui/accordion' and
remove (or replace) the line exporting './base-ui' so you only export the
intended modules directly (or only export the base-ui barrel), ensuring no
duplicate re-exports of ui/button and other identical identifiers.
| <AccordionPrimitive.Content | ||
| ref={ref} | ||
| className='overflow-hidden text-sm data-[state=closed]:animate-accordion-up data-[state=open]:animate-accordion-down' | ||
| {...props} | ||
| > | ||
| <div className={cn('pb-4 pt-0', className)}>{children}</div> | ||
| </AccordionPrimitive.Content> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major | ⚡ Quick win
Apply className to AccordionPrimitive.Content, not the inner wrapper.
Current placement changes consumer-facing styling semantics and blocks overrides on the content root.
Proposed fix
<AccordionPrimitive.Content
ref={ref}
- className='overflow-hidden text-sm data-[state=closed]:animate-accordion-up data-[state=open]:animate-accordion-down'
+ className={cn(
+ 'overflow-hidden text-sm data-[state=closed]:animate-accordion-up data-[state=open]:animate-accordion-down',
+ className
+ )}
{...props}
>
- <div className={cn('pb-4 pt-0', className)}>{children}</div>
+ <div className='pb-4 pt-0'>{children}</div>
</AccordionPrimitive.Content>🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@packages/shadcn/src/ui/accordion.tsx` around lines 43 - 49, The
consumer-facing className is being applied to the inner wrapper instead of the
content root; change the placement so AccordionPrimitive.Content receives the
merged classes (including 'overflow-hidden text-sm
data-[state=closed]:animate-accordion-up
data-[state=open]:animate-accordion-down' and the incoming className via cn) and
remove className from the inner div that currently wraps children so that
consumers can override the content root; update the component that renders
AccordionPrimitive.Content (keeping ref, {...props}, children) to merge
cn(className, ...) onto AccordionPrimitive.Content instead of the inner <div>.
1 participant
Motivation
shadcnAPI.buttonimplementation with a canonical re-export to simplify maintenance and dependency management.Description
apps/ui-layout/components/website/ui/button.tsximplementation withexport { Button, buttonVariants } from '@repo/shadcn'andexport type { ButtonProps } from '@repo/shadcn'to consume the shared package.apps/ui-layout/package.jsonto add new dependencies including@base-ui-components/react,@radix-ui/react-accordion,@radix-ui/react-collapsible, and@radix-ui/react-separatorto support the new shared components.packages/shadcn/package.jsondependencies to include@base-ui-components/reactand several@radix-uipackages and kepttailwind-mergeand other existing deps.packages/shadcn/src/base-ui/*placeholder adapter modules that re-export existinguicomponents or adapt@base-ui-components/react(notably an adapteraccordion.tsxwith aBaseUiAccordionexport).packages/shadcn/src/ui/accordion.tsximplementation and updatedpackages/shadcn/src/index.tsxandpackages/shadcn/src/ui/index.tsxto export the newaccordionand thebase-uiindex.Testing
Codex Task
Summary by CodeRabbit
New Features
Refactor